kind integrated test workflow MVP #1269

BenTheElder · 2020-01-18T01:02:34Z

implements #986

BenTheElder · 2020-01-18T01:02:40Z

/hold

k8s-ci-robot · 2020-01-18T01:02:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [BenTheElder]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

BenTheElder · 2020-01-18T01:03:38Z

hack/ci/e2e-k8s.sh

+#
+export KIND_BUILD_TYPE="${BUILD_TYPE:-bazel}"
+export KIND_TEST_FOCUS_OVERRIDE="${FOCUS:-}"
+export KIND_TEST_SKIP_OVERRIDE="${SKIP:-}"


in the future we will stop doing this and just control the lists in code centrally (IE kind test presubmit will have a fixed list, with the override being meant for local prototyping hacks, not CI)

hack/ci/e2e-k8s.sh

aojea · 2020-01-18T08:24:52Z

pkg/cmd/kind/test/internal/testcmd/testcmd.go

+	// In Kubernetes CI we don't have real IPv6 connectivity so we need to
+	// employ some work arounds when testing IPv6 clusters
+	// We do this all environments for consistency.
+	if config.Networking.IPFamily == v1alpha4.IPv6Family {


We should add a TODO for DualStack
config.Networking.IPFamily == v1alpha4.IPv6Family || config.Networking.IPFamily == v1alpha4.DualStackFamily

we can't realy TODO constants that don't exist yet, dual stack my not be a family, it will probably be many families?
dual stack as an API is a TODO.

also, there is validation long before this code rune that errors if not one of ipv4 / ipv6 specifically to handle the fact that this command is not aware of any other families currently

aojea · 2020-01-18T08:30:10Z

pkg/cmd/kind/test/internal/testcmd/testcmd.go

+		skip = strings.Join([]string{`\[Serial\]`, skip}, "|")
+	}
+	cmd := exec.Command(
+		"./hack/ginkgo-e2e.sh",


do we need the ./hack/ginkgo-e2e.sh bash script?
a quick look to the script seems that we can avoid most of the checks and run directly the e2e.testbinary,

that's orthogonal to this PR. but in any case I'm not convinced that this is useful right now, it will skew versus other test callers and this script also does helpful things like locating the e2e binary in the many possible build paths.

aojea · 2020-01-18T08:31:22Z

pkg/cmd/kind/test/internal/testcmd/testcmd.go

+	node := nodes[0]
+
+	// "fix" coreDNS configmap
+	// TODO: refactor this to not be a shell script.


+1000
This can also be a good /help wanted issue

maybe? probably not. the replacements need to be equivalent. i'm not sure how many casual contributors know sed / shell and go string manipulation deeply 🤷‍♂

aojea · 2020-01-18T08:34:51Z

pkg/cmd/kind/test/test.go

+		Args: cobra.NoArgs,
+		// TODO(bentheelder): more detailed usage
+		Use:   "test",
+		Short: "Tests one of [presubmit, conformance]",


how can I run my custom tests? this is specially useful to troubleshoot e2e tests or to create new ones, where you only want to run 1 or a small number of tests.
can we add one subcommand that allows to specify my custom FOCUS and SKIP fields?

see the e2e.sh for details on overriding these

it's not intended to run your tests. it's intended to run the standard flows in a way a non-power-user can trivially mimic by invoking the command(s). you as a power user can still write a little script with your flows.

I think I've found my answer, just let me rephrase, how can I specify FOCUS and SKIP with this change?
I have to use the env variables KIND_TEST_SKIP_OVERRIDE and KIND_TEST_FOCUS_OVERRIDE right?

those are temporary for transitioning the prowjobs
you can run e2e.test / ginkgo e2e.sh yourself currently.

aojea · 2020-01-18T08:39:57Z

pkg/cmd/kind/test/internal/testcmd/testcmd.go

+		// don't use the horrid e2e.test """provider""" concept
+		"--provider=skeleton",
+		// need to inform tests of worker count
+		"--num-nodes=", strconv.Itoa(numNodesForE2E),


the jobs are failing with

I0118 01:25:13.095] [1] invalid value "" for flag -num-nodes: strconv.ParseInt: parsing "": invalid syntax
I0118 01:25:13.095] [1] Usage of /go/src/k8s.io/kubernetes/bazel-bin/test/e2e/e2e.test:
🤷‍♂

ok, there is the problem, is adding a space

W0118 01:25:14.472] Call stack: W0118 01:25:14.472] 1: ./hack/ginkgo-e2e.sh:143 main(...) W0118 01:25:14.472] Exiting with status 1 W0118 01:25:22.159] ERROR: command "./hack/ginkgo-e2e.sh --provider=skeleton --num-nodes= 2 --report-dir /workspace/_artifacts --disable-log-dump=true --ginkgo.focus '' --ginkgo.skip '\[Serial\]|'" failed with error: exit status 1

no, the space is a rendering of passing this to the shell.
the problem is that we're passing it as two arguments, but the first contains =

marked the PR WIP, I haven't even run this yet. by inspection it's mostly done but it's still a prototype. I'm going to rework things a bit before un-WIP-ing it.

BenTheElder · 2020-01-23T02:51:31Z

/test pull-kind-e2e-kubernetes

BenTheElder · 2020-01-23T06:57:42Z

e2e is failing because kubectl isn't being found on the path when building with bazel + running ginkgo-e2e.sh. probably not actually fixed upstream :/
will settle on an option for this tomorrow.

aojea · 2020-01-23T07:32:54Z

pkg/cmd/kind/test/internal/testcmd/testcmd.go

+			return
+		} else if err2 != nil {
+			err = err2
+		} else {


this is only if err3 != nil or we alway assign err3 to err at this point?

we don't care at this point, because if err3 is nil at this point so are err2 and err /shrug

aojea · 2020-01-23T07:35:19Z

pkg/cmd/kind/test/internal/testcmd/testcmd.go

+	// build test binaries
+	switch buildType {
+	case "bazel":
+		if err := exec.InheritOutput(


this writes to the logger, right?

no. this wires it to stdout / stderr.

this respecting streams is a broad TODO, but it's not super important for cmd/ at the moment

aojea · 2020-01-23T07:38:47Z

pkg/cmd/kind/test/internal/testcmd/testcmd.go

+
+	"github.com/spf13/cobra"
+
+	"sigs.k8s.io/kind/pkg/apis/config/v1alpha4"


shouldn't we use the internal APIs?

no. internal apis are for:

translating between versions

allowing runtime code implementing the API to handle a single version, possibly with extra (computed?) data that is not part of the public API.

the CLI is a client of the runtime.

aojea · 2020-01-23T07:41:31Z

pkg/cmd/kind/test/internal/testcmd/testcmd.go

+}
+
+func enableIPv6Workarounds(provider *cluster.Provider, name string) error {
+	// get a control plane node to use run kubectl from


maybe we should check that ipv6 is enabled? or at least ipv6 forwarding?

the caller is responsible for having ipv6 enabled docker. we're not going to muck with the host in kind

aojea · 2020-01-23T07:50:20Z

pkg/cmd/kind/test/internal/testcmd/testcmd.go

+	if err != nil {
+		return errors.Wrap(err, "failed to get nodes for coreDNS IPv6 CI fixes")
+	}
+	nodes, err = nodeutils.ControlPlaneNodes(nodes)


nit, a different name to the assigned variable to distinguish from the previous one?

that's anti-helpful here. we don't want to retain and accidentally use the unfiltered node list.
we could break this out into a helper method but it's just a few lines so that's not super helpful

nah, no problem at all, is just there are at least 1 or 2 places more doing this that way, that's why I added the nit 😄

BenTheElder · 2020-01-23T23:09:28Z

/retest
/hold

k8s-ci-robot · 2020-02-10T23:57:54Z

@BenTheElder: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

aojea · 2020-03-03T08:35:58Z

Do you think that is possible to implement 2 executions of the e2e.test?
One using parallel without the serial tests and other only with the serial tests and aggregating the results of both runs

BenTheElder · 2020-03-04T05:05:08Z

I don't think we should deviate from the rest of upstream currently.

…

On Tue, Mar 3, 2020 at 12:35 AM Antonio Ojea ***@***.***> wrote: Do you think that is possible to implement 2 executions of the e2e.test? One using parallel without the serial tests and other only with the serial tests and aggregating the results of both runs — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1269?email_source=notifications&email_token=AAHADK454Y4KDLPLGLPCGY3RFS6O7A5CNFSM4KIQDUA2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENSSGOY#issuecomment-593830715>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHADK7GP37XHS7UHZJU6EDRFS6O7ANCNFSM4KIQDUAQ> .

BenTheElder · 2020-05-19T22:48:24Z

okaaay, let's see about getting this up to date again

fejta-bot · 2020-08-17T22:57:41Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

fejta-bot · 2020-12-24T20:15:59Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

fejta-bot · 2021-01-23T20:59:45Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

k8s-ci-robot · 2021-02-06T00:01:17Z

@BenTheElder: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kind-conformance-parallel-ipv6	`e445481`	link	`/test pull-kind-conformance-parallel-ipv6`
pull-kind-conformance-parallel-1-17	`e445481`	link	`/test pull-kind-conformance-parallel-1-17`
pull-kind-e2e-kubernetes-1-18	`e445481`	link	`/test pull-kind-e2e-kubernetes-1-18`
pull-kind-test	`e445481`	link	`/test pull-kind-test`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

fejta-bot · 2021-03-08T00:55:07Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

k8s-ci-robot · 2021-03-08T00:55:12Z

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 18, 2020

k8s-ci-robot requested review from aojea and munnerz January 18, 2020 01:02

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 18, 2020

BenTheElder commented Jan 18, 2020

View reviewed changes

BenTheElder force-pushed the test-commands branch from 265bef1 to 0dac43b Compare January 18, 2020 01:05

BenTheElder commented Jan 18, 2020

View reviewed changes

hack/ci/e2e-k8s.sh Outdated Show resolved Hide resolved

BenTheElder force-pushed the test-commands branch 2 times, most recently from 31dece3 to 7e40db8 Compare January 18, 2020 01:16

aojea reviewed Jan 18, 2020

View reviewed changes

BenTheElder changed the title ~~kind test MVP~~ [WIP] kind integrated test workflow MVP Jan 18, 2020

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 18, 2020

BenTheElder force-pushed the test-commands branch from 7e40db8 to 937309a Compare January 22, 2020 23:41

BenTheElder force-pushed the test-commands branch from 937309a to 6ac8487 Compare January 23, 2020 06:52

aojea reviewed Jan 23, 2020

View reviewed changes

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 30, 2020

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 10, 2020

This was referenced Mar 1, 2020

GA-only kind conformance test job kubernetes/test-infra#16560

Merged

Add GA_ONLY option to e2e-k8s script #1378

Merged

BenTheElder mentioned this pull request May 18, 2020

test IPVS mode #1607

Closed

This was referenced May 28, 2020

Add host path mount support for k8s e2e test script #1628

Closed

Revert "Revert "Rely on default watch cache capacity and ignore its requested size"" kubernetes/kubernetes#91491

Merged

BenTheElder mentioned this pull request Jun 19, 2020

[WIP] disable build with bazel in kubernetes CI #1675

Closed

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 17, 2020

BenTheElder mentioned this pull request Sep 11, 2020

use kube::util::find-binary to find kubectl #1844

Merged

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 16, 2020

BenTheElder removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Sep 25, 2020

kubernetes-sigs deleted a comment from fejta-bot Sep 25, 2020

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 24, 2020

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 23, 2021

k8s-ci-robot closed this Mar 8, 2021

BenTheElder mentioned this pull request Jul 29, 2021

Add ability to specify log format for componentManager and apiServer. #2387

Merged

BenTheElder mentioned this pull request Dec 18, 2022

Support an integrated test workflow #986

Open


		"github.com/spf13/cobra"

		"sigs.k8s.io/kind/pkg/apis/config/v1alpha4"

kind integrated test workflow MVP #1269

kind integrated test workflow MVP #1269

Conversation

BenTheElder commented Jan 18, 2020

BenTheElder commented Jan 18, 2020

k8s-ci-robot commented Jan 18, 2020

Choose a reason for hiding this comment

aojea Jan 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aojea Jan 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenTheElder commented Jan 23, 2020

BenTheElder commented Jan 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenTheElder commented Jan 23, 2020

k8s-ci-robot commented Feb 10, 2020

aojea commented Mar 3, 2020

BenTheElder commented Mar 4, 2020 via email

BenTheElder commented May 19, 2020

fejta-bot commented Aug 17, 2020

fejta-bot commented Dec 24, 2020

fejta-bot commented Jan 23, 2021

k8s-ci-robot commented Feb 6, 2021

fejta-bot commented Mar 8, 2021

k8s-ci-robot commented Mar 8, 2021

aojea Jan 18, 2020 •

edited

Loading

aojea Jan 18, 2020 •

edited

Loading